Codecov Report

Merging #401 (b838f0f) into master (b10ee0e) will increase coverage by 0.35%.
The diff coverage is 77.64%.

@@            Coverage Diff             @@
##           master     #401      +/-   ##
==========================================
+ Coverage   73.87%   74.22%   +0.35%     
==========================================
  Files          31       32       +1     
  Lines        4685     5378     +693     
  Branches     1467     1680     +213     
==========================================
+ Hits         3461     3992     +531     
- Misses        734      823      +89     
- Partials      490      563      +73     

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

This PR introduces a "disk-mode" for dqlite where the main SQLite database is
stored in a regular SQLite file on disk instead of storing the pages of the
database in memory.

disk-mode is considered experimental. Users can try it out and report any
issues with the new SQLite VFS or raft fsm.

Design considerations:

• The SQLite database files live in a folder database in the raft dir. This
  folder is emptied on every startup. The user is urged not to interact
  directly with the database file while the system is running.
  The database file is not the complete picture of the SQLite database. The
  WAL still lives in memory and is not persisted to disk.
• Snapshots are taken using the async snapshot interface of raft and are taken
  in 3 (simplified) steps.
  1. copy the WAL and disable checkpoints, this guarantees the contents of the
     disk file will not be overwritten during snapshotting.
  2. mmap the database files and write the snapshot to disk
  3. enable checkpoints again and clean up resources.

Known limitations:

• The blocking SQLite API calls are executed in the raft/dqlite main loop,
  which can lead to blocking the main event loop. One solution to fix this
  could involve the use of coroutines (to be investigated).
• Raft snapshots of the whole database are still loaded in memory in 1 chunck
  when restoring the database. This happens on startup or when installing a
  snapshot after receiving a raft InstallSnapshot RPC. So, while memory usage
  should decrease greatly when using this mode, large memory spikes will still
  occur. libraft will have to be adapted to allow.
  a) Sending a snapshot in chunks, with the receiver of the snapshot
  accumulating the chunks on disk.
  b) Restoring a snapshot in chunks.
  c) Taking a snapshot in chunks.

I believe supporting disk mode as a global on/off switch (vs a per-database one) is fine. We don't support multiple databases anyway at the moment I think.

I believe supporting disk mode as a global on/off switch (vs a per-database one) is fine. We don't support multiple databases anyway at the moment I think.

Thanks, I thought we did support multiple databases, I tested out a few cases and it appeared to be working (might have been a lucky shot, don't know). Will give it a closer look.

I've run a Jepsen test-suite run against this branch and noticed a - very - frequent failure, will try to figure it out first before merging this, will convert to Draft to inhibit accidental merge.

I believe supporting disk mode as a global on/off switch (vs a per-database one) is fine. We don't support multiple databases anyway at the moment I think.

Thanks, I thought we did support multiple databases, I tested out a few cases and it appeared to be working (might have been a lucky shot, don't know). Will give it a closer look.

It's kind of surprising, given the code here:

https://github.com/canonical/dqlite/blob/master/src/gateway.c#L130

but perhaps it works when using different connections.

I believe supporting disk mode as a global on/off switch (vs a per-database one) is fine. We don't support multiple databases anyway at the moment I think.

Thanks, I thought we did support multiple databases, I tested out a few cases and it appeared to be working (might have been a lucky shot, don't know). Will give it a closer look.

It's kind of surprising, given the code here:

https://github.com/canonical/dqlite/blob/master/src/gateway.c#L130

but perhaps it works when using different connections.

Indeed, when using multiple connections.

I've run a Jepsen test-suite run against this branch and noticed a - very - frequent failure, will try to figure it out first before merging this, will convert to Draft to inhibit accidental merge.

Could it be that the event look gets blocked for too long? For example when a checkpoint happens I'd expect the event loop to be blocked for a relatively long time, since we might need to write as many as 1000 pages to disk (the default checkpoint threshold is 1000).

Just thinking loud.

I've run a Jepsen test-suite run against this branch and noticed a - very - frequent failure, will try to figure it out first before merging this, will convert to Draft to inhibit accidental merge.

Could it be that the event look gets blocked for too long? For example when a checkpoint happens I'd expect the event loop to be blocked for a relatively long time, since we might need to write as many as 1000 pages to disk (the default checkpoint threshold is 1000).

Just thinking loud.

I first made the implementation with sync snapshots and later converted to async snapshots, but I did a sloppy job and raft copies the WAL in a uv worker thread while it is actively being written, while it should have copied the WAL in the main thread. Fixing it.

Jepsen tests are looking good, still investigating a failure that occurs with the disk-nemesis, possibly not a bug in this PR but in managing the behaviour when a raft node converts to RAFT_UNAVAILABLE after failing to apply a command to the state machine.

The remaining failure in the Jepsen suite is related to canonical/go-dqlite#213 . apply_frames fails because sqlite3_open_v2 fails due to the disk being full and the node becomes unavailable. This happens to every node in the cluster, resulting in an unavailable cluster.

I think this can be merged. The failure looks not to be related to this PR.